Skip to content

Support 3D Custom Switch Blocks #2370

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 108 commits into from
Sep 13, 2024
Merged

Support 3D Custom Switch Blocks #2370

merged 108 commits into from
Sep 13, 2024

Conversation

saaramahmoudi
Copy link
Contributor

@saaramahmoudi saaramahmoudi commented Aug 15, 2023

Description

This pull request support 3d custom switch blocks in the architecture file and automatically add the inter-die edges between tracks in different layer to RR graph.

Types of changes

  • Bug fix (change which fixes an issue)
  • New feature (change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed

@github-actions github-actions bot added VPR VPR FPGA Placement & Routing Tool libarchfpga Library for handling FPGA Architecture descriptions labels Aug 15, 2023
@saaramahmoudi saaramahmoudi force-pushed the 3d_track_to_track_conn branch from 1bfc97f to 99a7f2a Compare August 31, 2023 22:27
@@ -2461,6 +2461,13 @@ argparse::ArgumentParser create_arg_parser(const std::string& prog_name, t_optio
.default_value("false")
.show_in(argparse::ShowIn::HELP_ONLY);

route_grp.add_argument(args.custom_3d_sb_fanin_fanout, "--custom_3d_sb_fanin_fanout")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like it would fit better in the arch file (as a new parameter) than as a command line parameter. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also think that it might be better to put it in the architecture file, but for now I just keep it as it is to make the testing easier with command line option. When the branch is final, I will talk to you about the architecture change.

@@ -2333,7 +2338,7 @@ static void get_switchblocks_edges(RRGraphBuilder& rr_graph_builder,
++edge_count;

//we only add the following edge between intermediate nodes once for the first 3D connection for each pair of intermediate nodes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a big comment somewhere saying how custom_3d_sb_fanin_fanout works? It should detail exactly what the rr graph this generates looks like.

@vaughnbetz
Copy link
Contributor

Quite a few CI failures ... can you take a look @saaramahmoudi ?

@saaramahmoudi
Copy link
Contributor Author

saaramahmoudi commented May 21, 2024

Quite a few CI failures ... can you take a look @saaramahmoudi ?

Yes, I will. But before that, we still have some routing problem that might be related to router lookahead. I already talked to @amin1377, and he agreed to take a look into the new commits. After we found all bugs, I will investigate CI issues.

@vaughnbetz
Copy link
Contributor

Please go ahead and merge this after the conflicts are resolved, and a QoR test on Titan to ensure 2D QoR isn't affected.
There are two comments above that should go in an issue to resolve afterwards:

  • move --custom_3d_sb_fanin_fanout to the architecture file as a new tag, rather than as a command line option. It should go in the section.
  • Add a big comment somewhere (maybe in rr_graph.cpp) about how custom_3d_sb_fanin_fanout works. It should detail exactly what the rr graph this generates looks like.

@vaughnbetz
Copy link
Contributor

Issue #2694 is tracking the updates listed above.

@amin1377
Copy link
Contributor

amin1377 commented Sep 13, 2024

2D Titan QoR:
image
2D large VTR:
image

@@ -1015,6 +1019,10 @@ t_bb ConnectionRouter<Heap>::add_high_fanout_route_tree_to_heap(
if (!inside_bb(rr_node_to_add, net_bounding_box))
continue;

auto rt_node_layer_num = rr_graph_->node_layer(rr_node_to_add);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to comment what you are doing here and why.

@vaughnbetz
Copy link
Contributor

Looks good. One commenting update requested, plus we should add the missing comments I flagged before. I'll create an issue to track that.

@vaughnbetz
Copy link
Contributor

3D arch status:

  • Connecting across layers with OPINs (full or partial) works in this PR as well as it does on the master. @amin1377 please add data once you have it.
  • Connecting via 3D switch block does not work well
    • Router lookahead takes a long time to create (Amin thinks about 3x as long). The data in it looks reasonable though.
    • Router cannot resolve congestion.
    • Both issues exist on the master branch so we can merge anyway.

@vaughnbetz vaughnbetz merged commit 85c9928 into master Sep 13, 2024
36 of 52 checks passed
@vaughnbetz vaughnbetz deleted the 3d_track_to_track_conn branch September 13, 2024 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang-cpp C/C++ code libarchfpga Library for handling FPGA Architecture descriptions VPR VPR FPGA Placement & Routing Tool
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants